Check if merge is aborted during HNSW graph construction by jeho-rpls · Pull Request #16368 · apache/lucene

jeho-rpls · 2026-07-07T04:11:51Z

HNSW graph construction never checked OneMerge's abort flag. It is a pure-CPU loop that performs no writes, so IndexWriter#rollback and abortMerges() blocked until the entire graph was built. For production-sized vector segments that means tens of minutes (measurements in the issue).

Changes

HnswBuilder gains setAbortCheck(IORunnable), and HnswGraphBuilder invokes it before every node insertion in addGraphNodeInternal.
- This is the single point that all insertion paths funnel through: full rebuild via addVectors, graph join via MergingHnswGraphBuilder, and HnswConcurrentMergeBuilder workers (which forward the check).
Lucene99HnswVectorsWriter passes mergeState::checkAborted into the graph mergers (new constructor overloads, existing constructors unchanged).
Flush-time graph builds are unaffected (no check set), matching the null contract of checkIntegrity(OneMerge) from Check if merge is aborted during file integrity checksums #16281.
Overhead: one null check per node insertion, where each insertion costs hundreds of vector comparisons.

Testing

New TestHnswMergeAbort exercises all three merge paths end-to-end (rollback during a forceMerge of a 48k x 96d index). Without the fix, rollback blocks until the build finishes. With the fix, it returns in milliseconds.
- full rebuild (every segment above 40% deletions, so no base graph is eligible): blocked 27 s without the fix
- graph join (deletion-free segments, MergingHnswGraphBuilder): blocked 31 s
- concurrent merge (numMergeWorkers = 2, HnswConcurrentMergeBuilder): blocked 14 s
./gradlew tidy and :lucene:core:check pass.
The approach was also validated on a production Elasticsearch 9.4.2 cluster via a 10.4 backport.
- Node shutdown during a live HNSW merge went from 24–31 min (grace period exceeded, force-killed) to 2.2 s.
- Undisturbed merges complete unchanged (details in HNSW graph construction during merge never checks for abort, blocking IndexWriter#rollback for tens of minutes #16367).

Graph construction is a pure-CPU loop that performs no writes, so it never observed OneMerge's abort flag: IndexWriter#rollback/abortMerges blocked until the entire graph was built (tens of minutes for large vector segments). Wire MergeState#checkAborted from Lucene99HnswVectorsWriter through the HNSW graph mergers into the builders, and invoke it before every node insertion in HnswGraphBuilder#addGraphNodeInternal - the single point that all insertion paths (full rebuild, graph-join, concurrent workers) funnel through. Flush-time graph builds are unaffected (no check set).

jeho-rpls · 2026-07-07T04:22:25Z

The CI failure is unrelated to this change: TestForceNoBulkScoringQuery.testBulkScoringIsDisabled (lucene/monitor) fails on a clean main checkout (a9eb52b, no changes from this PR) with the same seed:

gradlew -p lucene/monitor test --tests TestForceNoBulkScoringQuery.testBulkScoringIsDisabled -Dtests.seed=BDD18A3ADD885193 -Dtests.locale=ii-CN -Dtests.timezone=Etc/Zulu

fails deterministically with Expected exception AssertionError but no exception was thrown (test environment: codec=CheapBastard). Re-pushed to re-trigger CI with fresh seeds.

jeho-rpls · 2026-07-07T05:50:32Z

Opened #16369 with a fix for the unrelated test failure above.

Add a variant with numMergeWorkers=2 so the abort check forwarding in HnswConcurrentMergeBuilder is exercised, and relax the rollback bound from 5s to 10s for slow CI machines. Without the fix the three variants block rollback for 27s (full rebuild), 31s (graph join) and 14s (concurrent) on this index.

# Conflicts: # lucene/CHANGES.txt

msokolov · 2026-07-10T22:29:03Z

Is this really needed? When do we ever call abortMerges()? It looks to me as if it gets called when an exception happens while closing an IndexWriter, or explicitly rolling back. Seem like kind of exceptional cases, although maybe we would want rollbacks to be faster? Do other merge operations check for aborted status?

jeho-rpls · 2026-07-11T09:08:50Z

Hi @msokolov, thanks for taking a look!

Is this really needed? When do we ever call abortMerges()? It looks to me as if it gets called when an exception happens while closing an IndexWriter, or explicitly rolling back. Seem like kind of exceptional cases

abortMerges() is routine at the deployment level even though it looks exceptional at the API level.
Elasticsearch closes its shard engines through IndexWriter#rollback on node shutdown, so every rolling restart of a data node goes through it.

That is how we hit this.

When a node is asked to shut down while an HNSW merge is in the middle of graph construction, that rollback blocks until the build completes. In containerized deployments it easily outlives the pod termination grace period, so the process ends up force-killed mid-merge.
In our case, a routine rolling restart of a vector-heavy cluster blocked for 24–31 minutes per node behind a single in-flight merge, past the 30-minute pod grace in the worst case (measurements in HNSW graph construction during merge never checks for abort, blocking IndexWriter#rollback for tens of minutes #16367).

Elasticsearch reports hitting a similar issue in production and has an open downstream workaround (elastic/elasticsearch#152342), but it lives in their stateless plugin only, and the workaround's author notes it is partial coverage. A check in the graph builder covers every directory implementation.

although maybe we would want rollbacks to be faster?

Yes, exactly. Faster rollback is the whole motivation. The wait also buys nothing. The writer is rolling back, so the graph being built is discarded the moment it completes.

Do other merge operations check for aborted status?

They do.

Writes: every merge output is wrapped via ConcurrentMergeScheduler#wrapForMerge into RateLimitedIndexOutput, and MergeRateLimiter#maybePause throws MergeAbortedException once the flag is set. This is the "they periodically check if they are aborted" that abortMerges relies on.
Checksums: since Check if merge is aborted before executing file integrity checks #16264 and Check if merge is aborted during file integrity checksums #16281, every format consumer calls MergeState#checkAborted before each producer's integrity check, and CodecUtil#checksumEntireFile(input, merge) re-checks the flag every megabyte read.

Graph construction is the odd one out. It performs no writes and no checksum reads for tens of minutes, so it never observes the flag. In our profiling it was 93.6% of the merge CPU, while the checksum phase covered by #16281 was 0.1%.

msokolov

OK, this makes sense to me; let's resolve the one comment and this looks good to merge

msokolov · 2026-07-12T22:05:17Z

+   * org.apache.lucene.index.MergePolicy.MergeAbortedException} when the merge that triggered the
+   * build has been aborted.
+   */
+  void setAbortCheck(IORunnable abortCheck);


I don't like that we could set it, unset it, set it to something different, all while merging. Ideally it would be final and set in constructor, although with so many overrides that looks a little gross. Maybe we could at least throw an exception in implementations of this method if it is non-null, so it is set-once?

Thanks for the suggestion.
2456582 : setAbortCheck now throws IllegalStateException on a second set and rejects null. Added a test.

Thanks, @jeho-rpls, I'll merge now

jeho-rpls · 2026-07-14T04:27:15Z

@msokolov Thanks for the review and merge!
Could this be backported to branch_10x for 10.6? The new API surface is confined to the @lucene.experimental HnswBuilder interface and internal constructor overloads. Happy to open the cherry-pick PR.

msokolov · 2026-07-14T11:44:39Z

I guess I was sort of hoping we would get to an 11 release soon, but yes, feel free to open the backport. Don't forget to also move the CHANGES entry to the 10.6 section

msokolov · 2026-07-14T13:33:59Z

      double wallClockMs = (System.nanoTime() - mergeStartTimeNs) / 1_000_000.0;
      double totalWorkerMs = cumulativeWorkTimeNs.get() / 1_000_000.0;
      double effectiveConcurrency = wallClockMs > 0 ? totalWorkerMs / wallClockMs : 0;
      infoStream.message(


I was just wondering what happens if the merge aborts the worker threads here? Do we log "merge completed"?

Thanks for the question :)

No. The abort check throws MergeAbortedException inside the workers. TaskExecutor#invokeAll then cancels the remaining tasks, waits for the running ones to settle, and rethrows the first exception.
So build() exits at the invokeAll call and never reaches the "merge completed" line.
Each running worker also stops at its own next node insertion since they all share the same check.

With InfoStream enabled, an aborted concurrent merge logs only the initial "build graph" line. testRollbackDuringConcurrentMerge in this PR exercises the abort path.

(cherry picked from commit 2b23728)

jeho-rpls · 2026-07-14T16:35:41Z

Opened the backport at #16391. 🙏

…6391)

jeho-rpls · 2026-07-14T17:40:11Z

Opened the follow-up to move the main CHANGES entry: #16393 :)

github-actions Bot added module:core/index module:core/codecs module:core/hnsw labels Jul 7, 2026

jeho-rpls force-pushed the hnsw-merge-abort-16367 branch from 5e9919a to f7608f1 Compare July 7, 2026 04:22

jeho-rpls mentioned this pull request Jul 7, 2026

Fix seed-dependent failure in TestForceNoBulkScoringQuery #16369

Merged

jeho-rpls and others added 3 commits July 8, 2026 12:31

Merge branch 'main' into hnsw-merge-abort-16367

644d4a2

Merge remote-tracking branch 'origin/main' into hnsw-merge-abort-16367

e812494

# Conflicts: # lucene/CHANGES.txt

Merge branch 'main' into hnsw-merge-abort-16367

c3ee5c7

msokolov reviewed Jul 12, 2026

View reviewed changes

Enforce that the abort check is set at most once

2456582

msokolov merged commit 2b23728 into apache:main Jul 13, 2026
12 checks passed

msokolov reviewed Jul 14, 2026

View reviewed changes

jeho-rpls mentioned this pull request Jul 14, 2026

Check if merge is aborted during HNSW graph construction (#16368) #16391

Merged

jeho-rpls added a commit to jeho-rpls/lucene that referenced this pull request Jul 14, 2026

Check if merge is aborted during HNSW graph construction (apache#16368)

41f256c

(cherry picked from commit 2b23728)

msokolov pushed a commit that referenced this pull request Jul 14, 2026

Check if merge is aborted during HNSW graph construction (#16368) (#1…

2cae9d5

…6391)

tlrx mentioned this pull request Jul 15, 2026

Abort stateless merge reads on shard close to unblock HNSW merges elastic/elasticsearch#152342

Open

kaivalnp mentioned this pull request Jul 15, 2026

Test suite TestHnswMergeAbort slows down build #16397

Closed

ChrisHegarty added this to the 10.6.0 milestone Jul 22, 2026

Uh oh!

Conversation

jeho-rpls commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Testing

Uh oh!

jeho-rpls commented Jul 7, 2026

Uh oh!

jeho-rpls commented Jul 7, 2026

Uh oh!

msokolov commented Jul 10, 2026

Uh oh!

jeho-rpls commented Jul 11, 2026

Uh oh!

msokolov left a comment

Choose a reason for hiding this comment

Uh oh!

msokolov Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

jeho-rpls Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

msokolov Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jeho-rpls commented Jul 14, 2026

Uh oh!

msokolov commented Jul 14, 2026

Uh oh!

msokolov Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

jeho-rpls Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

jeho-rpls commented Jul 14, 2026

Uh oh!

jeho-rpls commented Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jeho-rpls commented Jul 7, 2026 •

edited

Loading